Add community knowledge: AL boolean operators do not short-circuit - #136
Conversation
AL gives no short-circuit (lazy) evaluation guarantee for and/or/xor — neither the AL operators nor the boolean operators documentation defines a lazy evaluation order. LLMs trained on C#, JavaScript, or SQL assume the left operand guards the right, which produces conditions where a guard does not protect an unsafe subscript or a field read after a failed Get, and where an expensive operand is paid on every path. Adds community/knowledge/performance/boolean-operators-do-not-short-circuit.md with good/bad AL companions. The guidance prefers nested if when one operand depends on another, while keeping and/or legitimate for operands that are independently safe and cheap, so a reviewer does not flag harmless bound checks. This is the first article in a community performance domain; the Microsoft performance review leaf skill already sources candidates by domain across every enabled layer, so no skill change is needed to reach it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Some people prefer to use the in operator to short-circuit/lazy evaluate but AL developers are most used to seeing if statements and the in operator might confuse more than help, at least in the beginning, so I added nested if conditions instead in the knowledge file and good pattern. |
Natalie Karolak, MVP (NKarolak)
left a comment
There was a problem hiding this comment.
Nice one!
Mind that for a long enumeration of if to follow, the case true of or case false of pattern might be interesting as well.
https://nataliekarolak.wordpress.com/2022/12/21/lazy-evaluation-in-al/
|
Great suggestion, will add that so that we don't end up with more than 3 nested if statements. I'm used to use the in operator (same thing as the case true/false of) but I agree that developers will feel/be more comfortable or at home with the case of. |
Follow-up from PR review: nested if is the right answer for two or three dependent conditions, but past that the nesting becomes the problem. AL's case statement is the flat alternative — the control statements documentation states a value set "must be an expression or a range" and that the first matching value set executes, so case true of / case false of accept boolean expressions and stop at the first match. That is the laziness the boolean operators do not provide. Adds case-true-of-for-long-condition-chains.md with good/bad AL companions: case false of for guard chains where every condition must hold, case true of for first-match dispatch. The bad sample shows both failure shapes — a five-level if ladder, and the worse escape of collapsing it into an and chain, which trades nesting for a real defect. Cross-links both articles, and adds the threshold to the short-circuit article's Best Practice so following it does not lead to a deep ladder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback: the five value sets sharing exit(false) should be comma separated. Applied to the three that are pure field reads with no order dependency. The Get and the Blocked read keep their own value sets. The documentation guarantees that the first matching value set executes, which orders matching across separate value sets; it says nothing about evaluation within one comma-separated set, and the natural lowering of that is an equality-or chain — where AL's or does not short-circuit. Grouping the Get with the checks that must precede it would rest the sample's correctness on undocumented behaviour, which is the defect these two articles exist to prevent. Encodes the boundary in the Best Practice section so the grouping is applied where it is safe and not where it is not, and scopes the repeated-exit detection signal explicitly to nested chains so the case sample does not read as its own anti-pattern. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review decision: all five conditions share one action, so they share one comma-separated value set with a single exit(false). Aligns the article's Best Practice with the sample — it previously told authors to keep side-effecting conditions in their own value set, which the sample no longer does — and drops the now-contradictory wording about listing the failure action per condition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Natalie Karolak, MVP (NKarolak)
left a comment
There was a problem hiding this comment.
One more reason why I like the case true/false of pattern a lot - you don't need all these parentheses ;-)
Applies NKarolak's review suggestion: a case value set needs no parentheses around a comparison. Encodes the rationale in the Best Practice section, since it is a real advantage of the pattern and is not documented elsewhere in the repo. The AL operator hierarchy places and/xor above the comparison operators and or just above them too, so parentheses are mandatory in an and chain — A = B and C = D misparses without them — while a case value set has no and to bind tighter and needs none. That inverts the precedence most developers arrive with from C#. Also notes in the bad sample that its parentheses are not optional, so the two samples contrast on parentheses as well as on evaluation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
Two correctness boundaries need tightening before this becomes agent guidance.
-
boolean-operators-do-not-short-circuit.mdcoversand,or, andxor, but its fix says to split dependent operands into nestedifstatements. That preserves anandcondition, not anorcondition: rewritingif A or ExpensiveB then DoSomething()as nestedifs changes the result whenAis true. Please giveorits own equivalent pattern (for example, an early-return helper orelse ifthat evaluates the right operand only when the left is false), and clarify thatxorinherently needs both operands. Anorfixture would keep future agents from applying theandrewrite mechanically. -
case-true-of-for-long-condition-chains.mdderives stop-at-first-expression behavior for one comma-separated value set from Microsoft's statement that the first matching value set executes. The documentation does not specify evaluation order within a value set, while the good sample depends onItem.Get(...)stopping evaluation beforeItem.Blocked. Community runtime demonstrations support that behavior, but the article should cite that empirical basis and avoid presenting the official wording as a guarantee it does not make, or use a structure whose ordering the docs explicitly guarantee.
The core strict-evaluation warning and the individual case true of first-match dispatch pattern are otherwise sound.
Fixes two points from JesperSchulz's review on PR microsoft#136. 1. boolean-operators-do-not-short-circuit.md gave one fix — nested if — for and, or, and xor alike. That's only correct for and: nesting if A then if B then Action drops the A-true/B-false case of A or B, silently changing the result. Gives or its own early-exit pattern (if A then exit(true); exit(B)), warns explicitly against applying the and-rewrite to or, and clarifies that xor is not a short-circuit candidate in any language since its result always depends on both operands. Adds an or fixture (IsEligibleForFreeShipping) to both samples so an agent has a concrete pattern to match instead of extrapolating from the and-only examples. 2. case-true-of-for-long-condition-chains.md derived stop-at-first-match for one comma-separated value set from the documentation's guarantee about the first matching value set — plural, i.e. ordering across value sets, which is not the same claim. The good sample's Item.Get / Blocked pair depended on the one the docs don't make. Restructures the sample to only comma-group the three pure, order-independent checks; Get and Blocked keep their own value sets, in order, relying solely on the guarantee that is actually documented. Description and Anti Pattern now state that boundary so it isn't re-collapsed later. Also carries forward a parenthesis fix (not Item.Blocked in the collapsed bad sample) that was made two commits ago but never landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Jesper Schulz-Wedde (@JesperSchulz) Both points landed in 4700047 — thanks, these were real gaps. 1. or/xor. You're right that the nested- 2. case value-set ordering. Agreed the good sample was resting on more than the docs state. Went with your second option: restructured rather than cited. PR description is updated to match. Let me know if either fix needs another pass. |
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
The follow-up resolves both correctness concerns.
and,or, andxornow have distinct, semantics-preserving guidance, with a concreteorfixture preventing agents from applying the nested-ifrewrite incorrectly.- The
case false ofsample now groups only pure, order-independent checks and placesGetand the dependent field read in separate value sets, relying only on the ordering Microsoft documents.
The resulting articles and samples are coherent and safe for agent consumption.
c39f723
into
microsoft:main
What
Two community knowledge articles under
community/knowledge/performance/, each with.good.al/.bad.alcompanions:boolean-operators-do-not-short-circuit.md— AL gives no short-circuit (lazy) evaluation guarantee forand,or, orxor.case-true-of-for-long-condition-chains.md—case true/false ofas the flat alternative once anand-guard chain runs past about three nestedifs.Why this passes the admission test
An LLM trained mostly on C#, JavaScript, or SQL carries short-circuiting in as a default assumption, and BC has no compiler diagnostic that contradicts it. That produces concrete failure shapes the two files prevent:
(Index >= 1) and (Index <= ArrayLen(Thresholds)) and (Amount > Thresholds[Index])still evaluates the subscript.Customer.Get(No) and (Customer.Blocked <> ...)readsBlockedfrom a record that was never loaded — a silently wrong result, not an error.and/orwhose left operand already determines the result.andfix applied toor. Nestingif A then if B then Actionis the correct rewrite ofA and B, but rewritingA or Bthe same way silently drops theA-true/B-false case. The first article givesorits own early-exit pattern (if A then exit(true); exit(B);) and warns explicitly against reusing theandrewrite.xorgets a separate note: it isn't a short-circuit candidate in any language, since its result always depends on both operands — the only guidance there is to keep both operands cheap.ifs deep, sequencing guards with noelse, is what the second article replaces with a flatcase false of/case true of— one indentation level, explicit ordering, no parentheses around a comparison the way anand/oroperand needs one.Note on the documentation
Neither AL operators nor Boolean operators mentions short-circuit or lazy evaluation in either direction, so the first article is worded as no guarantee is given, so do not rely on one rather than asserting a documented order. AL control statements does state that a
casevalue set "must be an expression or a range" and that "the first matching value set executes" — an actual documented guarantee, which the second article'scasepattern relies on. That guarantee is scoped to ordering across separate value sets; it says nothing about the order of expressions listed inside one comma-separated value set. The good sample only comma-groups the checks that are pure and order-independent (a field comparison, an enum test), and keeps a guard (Item.Get) and the condition it protects (Item.Blocked) in their own separate value sets — so the sample's correctness rests only on the guarantee the docs actually make.This — no official guarantee for the first article, and a guarantee that has to be applied carefully rather than assumed for the second — is part of why both went to
/community/rather than/microsoft/.Retrieval
Both files are
domain: performance. This is the first community-layer content in that domain;microsoft/skills/review/al-performance-review.mdalready sources candidates by domain across every enabled layer, so no skill change is needed to reach either article.Validation
python .github/scripts/validate_frontmatter.py --root .→ 0 errors, 0 warnings on both articles. Each is 30-36 lines, one concern, no fenced code blocks, every sample referenced by filename and present.evaluation/review-fixtures.jsonis untouched —performanceis pinned touse-isempty-for-existence-check.Review history
case true/false offor longand-guard chains → added as the second article.casevalue sets, since a value set needs none the way anand/oroperand does → applied, and the reasoning (AL's operator hierarchy) is now in the article's Best Practice.iffix in the first article doesn't generalize toor, and that the second article's good sample depended on an evaluation-order guarantee the docs don't make → both fixed as described above.🤖 Generated with Claude Code